feat(cli): add /model --compaction for configurable chat compression model#6019
feat(cli): add /model --compaction for configurable chat compression model#6019Rajeshwaran-R wants to merge 6 commits into
Conversation
Chat compression now uses config.getFastModel?.() (e.g., Qwen/Qwen3-Alpha) instead of the main model for compression side queries. This reduces cost while maintaining functionality. - Removed `model` parameter from compress() method signature - Pass config.getFastModel?.() to runSideQuery for compression - Update test to verify fast model is passed correctly - Test renamed: 'forwards model...' → 'passes getFastModel to runSideQuery for compression'
…model Introduce dedicated compaction model setting (configurable via /model --compaction) with fallback chain: compactionModel → fastModel → main model. Adds UI dialog mode, slash command flag, and runtime snapshot guard so compaction models are never presented as runtime selections.
doudouOUC
left a comment
There was a problem hiding this comment.
Additional findings (not mappable to diff lines):
[Suggestion] ModelDialog.tsx — title missing isCompactionModelMode branch. The dialog title conditional has branches for voice, vision, and fast modes but no isCompactionModelMode branch. When the compaction model picker opens, it shows the generic "Select Model" title instead of "Select Compaction Model", making it look like the user is changing the primary model.
[Suggestion] ModelDialog.tsx — no preferredCompactionModelEntry in the preferredKey chain. The currently configured compaction model is not highlighted when the dialog opens (unlike fast, voice, and vision modes which each have a preferred*ModelEntry computation).
| purpose: 'chat-compression', | ||
| skipOutputLanguagePreference: true, | ||
| model, | ||
| model: config.getFastModel?.(), |
There was a problem hiding this comment.
[Critical] config.getFastModel?.() should be config.getCompactionModel()
The PR adds getCompactionModel() on Config (with the correct fallback chain: compactionModel → fastModel → main model), but the compression service bypasses it entirely and hard-codes the fast model. Setting /model --compaction <id> persists the value and syncs it to runtime Config, but compression always uses the fast model — the feature is dead code end-to-end.
| model: config.getFastModel?.(), | |
| model: config.getCompactionModel(), | |
| // Compression uses the compaction model (falls back to fast model, then main model). | |
| // See https://github.com/QwenLM/qwen-code/issues/5956 |
Also update the test in chatCompressionService.test.ts to mock getCompactionModel instead of getFastModel.
— qwen3.7-max via Qwen Code /review
| (isVoiceModelMode || !m.voiceOnly), | ||
| (isVoiceModelMode || !m.voiceOnly) && | ||
| (isVisionModelMode || !m.visionOnly) && | ||
| (isCompactionModelMode || !m.visionOnly), |
There was a problem hiding this comment.
[Critical] Copy-paste bug: (isCompactionModelMode || !m.visionOnly) should be (isCompactionModelMode || !m.voiceOnly)
The compaction filter reuses !m.visionOnly from the vision line above. But getAvailableModelIds (line 219) and the CLI validation (line 613) both use !m.voiceOnly for compaction mode (same as fast mode). The dialog and CLI will show different model sets: the dialog excludes vision-only models (irrelevant to text compression) but includes voice-only models (unsuitable for compression).
| (isCompactionModelMode || !m.visionOnly), | |
| (isCompactionModelMode || !m.voiceOnly), |
— qwen3.7-max via Qwen Code /review
| isVoiceModelMode, | ||
| isVisionModelMode, | ||
| isCompactionModelMode, | ||
| }: ModelDialogProps): React.JSX.Element { |
There was a problem hiding this comment.
[Critical] handleSelect is missing an isCompactionModelMode handler block
The callback has handlers for isVoiceModelMode, isFastModelMode, and isVisionModelMode but no if (isCompactionModelMode) branch. When a user opens the compaction model picker via /model --compaction (no model-id) and selects a model from the dialog, the code falls through to the main model selection path (config.switchModel(...)), silently changing the user's primary model instead of setting the compaction model.
Add an if (isCompactionModelMode) block mirroring the isFastModelMode pattern:
- Encode the selector via
encodeAuxModelSelector(selected) - Persist via
settings.setValue(scope, 'compactionModel', fastModel) - Call
config?.setCompactionModel(fastModel) - Add a history item and
return
Also add isCompactionModelMode to the handleSelect dependency array (currently missing — the callback won't re-create when compaction mode activates, causing a stale closure).
— qwen3.7-max via Qwen Code /review
| expect(mockGenerateText).toHaveBeenCalledWith( | ||
| expect.objectContaining({ | ||
| model: mockModel, | ||
| model: 'fast-model-v1', |
There was a problem hiding this comment.
[Critical] Test verifies the wrong behavior
This test was renamed from "forwards model, maxAttempts, and thinkingConfig to runSideQuery" to "passes getFastModel to runSideQuery for compression" and asserts model: 'fast-model-v1'. But the compression service should use config.getCompactionModel() (which falls back to the fast model), not config.getFastModel() directly.
After fixing the service to use getCompactionModel():
- Mock
getCompactionModelreturning'compaction-model-v1'and assert it's used - Add a fallback test: when
getCompactionModelreturns undefined, assert the fast model is used
— qwen3.7-max via Qwen Code /review
| promptId, | ||
| force, | ||
| model, | ||
| config, |
There was a problem hiding this comment.
[Suggestion] CompressOptions.model is now a dead interface field
The model field remains required (model: string) in the CompressOptions interface and all callers in geminiChat.ts still compute and pass it, but compress() no longer destructures or uses it (the diff removed model from the destructure at line 319). This is a misleading API contract — callers believe they're controlling the compression model, but the value is silently discarded.
Either remove model from CompressOptions and update all callers, or (if keeping the interface for future use) mark it as @deprecated with a comment.
— qwen3.7-max via Qwen Code /review
| fastOnly?: boolean; | ||
| /** When true, this model only appears in the voice model selector */ | ||
| voiceOnly?: boolean; | ||
| /** When true, this model only appears in the vision model selector */ |
There was a problem hiding this comment.
[Critical] visionOnly is never propagated — the feature is dead on arrival
This PR adds visionOnly?: boolean to AvailableModel and uses it in ModelDialog filters (lines 257-258), but modelRegistry.ts was never updated to propagate the field from model configs. The registry only copies fastOnly and voiceOnly (modelRegistry.ts:233-234). Since no code ever sets visionOnly on a model, m.visionOnly is always undefined and !undefined is true — both vision mode and compaction mode filters that reference visionOnly have zero effect.
Add visionOnly: model.visionOnly, to modelRegistry.ts alongside the existing fastOnly/voiceOnly propagation, and add validation to reject models with both visionOnly and other flags set (mirroring the existing fastOnly + voiceOnly check at modelRegistry.ts:332).
— qwen3.7-max via Qwen Code /review
| }; | ||
| } | ||
|
|
||
| persistSetting(settings, 'compactionModel', modelName); |
There was a problem hiding this comment.
[Suggestion] Zero test coverage for /model --compaction command handler
85+ lines of new command handling code with no tests. The existing --fast and --vision flags have extensive test coverage that should be mirrored here. Missing tests for:
- Setting a valid compaction model:
/model --compaction <model-id> - Rejecting unavailable compaction models (both bare and auth-qualified)
- Non-interactive mode showing current compaction model status
- Dialog action when
/model --compactionis run with no args in interactive mode Config.getCompactionModel()/setCompactionModel()/resolveCompactionModelSelector()methodsisCompactionModelModestate management inuseModelCommand
— qwen3.7-max via Qwen Code /review
DragonnZhang
left a comment
There was a problem hiding this comment.
2 Critical findings in ModelDialog.tsx: (1) the new (isCompactionModelMode || !m.visionOnly) filter condition ANDs with the existing vision-mode filter, blocking vision-only models from the /model --vision list; (2) handleSelect lacks an isCompactionModelMode early-return path, causing model selection in compaction mode to call config.switchModel() and overwrite the primary model instead of persisting a compaction model.
— claude-sonnet-4-6 via Qwen Code /review
Generated by Claude Code
There was a problem hiding this comment.
[Critical] The newly added filter condition (isCompactionModelMode || !m.visionOnly) silently prevents vision-only models from appearing in the /model --vision dialog.
availableModelEntries filters registry models with two back-to-back conditions:
(isVisionModelMode || !m.visionOnly) && // ← existing line: shows all models in vision mode
(isCompactionModelMode || !m.visionOnly), // ← new line added by this PRBoth conditions are ANDed. When isVisionModelMode = true and isCompactionModelMode = false (i.e., the user runs /model --vision):
- Condition 1 evaluates to
true || !m.visionOnly=true(correct — include vision-only models) - Condition 2 evaluates to
false || !m.visionOnly=!m.visionOnly(wrong — excludes vision-only models)
The combined AND result is !m.visionOnly, so vision-only models can never appear in the selection list, defeating the purpose of condition 1. The fix is either to remove the new line (since condition 1 already handles the vision filter) or to add isVisionModelMode to the new guard:
(isCompactionModelMode || isVisionModelMode || !m.visionOnly),— claude-sonnet-4-6 via Qwen Code /review
Generated by Claude Code
There was a problem hiding this comment.
[Critical] handleSelect has no isCompactionModelMode branch. Selecting a model while in compaction mode falls through to config.switchModel(), which switches the primary chat model instead of persisting a compaction model.
The fast/voice/vision handlers each early-return with mode-specific persistence (settings.setValue(scope, 'fastModel', ...), 'visionModel', etc.). The compaction mode is missing an equivalent:
if (isCompactionModelMode) {
const compactionModel = encodeAuxModelSelector(selected);
const scope = getPersistScopeForModelSelection(settings);
settings.setValue(scope, 'compactionModel', compactionModel);
config?.setCompactionModel?.(compactionModel);
uiState?.historyManager.addItem({ type: 'success', text: `Compaction Model: ${compactionModel}` }, Date.now());
onClose();
return;
}Without this, /model --compaction + Enter overwrites the user's primary model setting, which is a destructive side effect of what should be a narrowly-scoped compaction model selection.
— claude-sonnet-4-6 via Qwen Code /review
Generated by Claude Code
wenshao
left a comment
There was a problem hiding this comment.
[Suggestion] Missing compactionModel in packages/vscode-ide-companion/schemas/settings.schema.json
The PR adds compactionModel to settingsSchema.ts but doesn't regenerate the committed schema JSON. fastModel and visionModel entries exist; compactionModel is missing. Run npm run generate:settings-schema and include the updated file.
— qwen3.7-max via Qwen Code /review
| get description() { | ||
| return t( | ||
| 'Switch the model for this session (--fast for suggestion model, --voice for voice transcription model, --vision for the vision bridge model, [model-id] to switch immediately).', | ||
| 'Switch the model for this session (--fast for suggestion model, --voice for voice transcription model, --vision for the vision bridge model, --compaction for chat compression model, [model-id] to switch immediately).', |
There was a problem hiding this comment.
[Critical] i18n test failure — missing translations for new description
The new description string with --compaction for chat compression model has no entries in zh.js or zh-TW.js. Both are strict-parity locales, so mustTranslateKeys.test.ts fails, blocking CI.
| 'Switch the model for this session (--fast for suggestion model, --voice for voice transcription model, --vision for the vision bridge model, --compaction for chat compression model, [model-id] to switch immediately).', | |
| 'Switch the model for this session (--fast for suggestion model, --voice for voice transcription model, --vision for the vision bridge model, --compaction for chat compression model, [model-id] to switch immediately).', |
Add the translation key to both Chinese locale files (packages/cli/src/i18n/locales/zh.js and zh-TW.js).
— qwen3.7-max via Qwen Code /review
| ? this.getAllConfiguredModels([selector.authType]) | ||
| : this.getAllConfiguredModels(); | ||
| if (!available.some((m) => m.id === selector.modelId)) { | ||
| return undefined; |
There was a problem hiding this comment.
[Suggestion] getCompactionModel() returns undefined instead of falling back when configured model is unavailable
When compactionModel is set but the model is no longer available (e.g., provider changed, credentials expired), this returns undefined instead of falling through to getFastModel() ?? getModel(). The documented priority chain (compactionModel → fastModel → main model) breaks at the first step.
| return undefined; | |
| return this.getFastModel() ?? this.getModel(); |
— qwen3.7-max via Qwen Code /review
| isFastModelMode, | ||
| isVoiceModelMode, | ||
| isVisionModelMode, | ||
| isCompactionModelMode, |
There was a problem hiding this comment.
[Suggestion] Three isCompactionModelMode integration gaps in this component
isCompactionModelMode is destructured here but not used in several places where the other mode flags are checked:
-
Dialog title (~line 790): No
isCompactionModelModebranch in the title ternary — falls through to generic "Select Model" instead of "Select Compaction Model". -
Left-arrow escape (~line 491):
useKeypresshandler checksisFastModelMode || isVoiceModelMode || isVisionModelModebut omitsisCompactionModelMode— left arrow won't dismiss the compaction dialog. -
Preferred entry (~line 430-483): No
preferredCompactionModelEntrycomputed — dialog opens with cursor at the wrong position instead of highlighting the currently configured compaction model.
Each should mirror the pattern used by isFastModelMode/isVisionModelMode.
— qwen3.7-max via Qwen Code /review
| return { | ||
| type: 'message', | ||
| messageType: 'info', | ||
| content: t( |
There was a problem hiding this comment.
[Suggestion] Misleading "clear" instruction — no clear mechanism exists
This message tells users to use "/model --compaction " to clear the override, but when an empty/whitespace model name is passed, the code only shows the info message or opens the dialog — it never calls setCompactionModel(undefined) or persistSetting(settings, 'compactionModel', '').
Either implement the clear path (mirroring how --fast could work), or remove the false instruction from this message.
— qwen3.7-max via Qwen Code /review
|
@qwen-code /resolve |
Resolved merge conflicts between the --compaction feature and main: - Keep both formatUnavailableCompactionModelMessage (PR) and formatAmbiguousVisionModelMessage (main) as independent new functions - Combine --compaction flag and inline prompt support in the /model command description and argumentHint - Keep both compaction model methods (PR) and reasoning effort methods (main) as independent new methods on Config - Update test assertion to match the merged description string Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
Qwen Code resolved the merge conflicts and pushed the branch update. Merge Conflict Resolution Summary — PR #6019PR: feat(cli): add /model --compaction for configurable chat compression model Conflicts Resolved1.
|
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
[Suggestion] ModelDialog.tsx:506 — Left-arrow dismiss missing isCompactionModelMode. The escape/left-arrow handler (isFastModelMode || isVoiceModelMode || isVisionModelMode) omits isCompactionModelMode, breaking the established UX shortcut for dismissing specialized model pickers.
[Suggestion] ModelDialog.tsx:440 — No preferredCompactionModelEntry. The preferredKey resolution chain walks activeRuntimeSnapshot → preferredVoiceModelEntry → preferredVisionModelEntry → preferredFastModelEntry → default with no branch for compaction. The current compaction model is never highlighted in the picker, giving users no visual feedback about what is already configured.
— qwen3.7-max via Qwen Code /review
| }; | ||
| } | ||
|
|
||
| const isCompactionModelCommand = |
There was a problem hiding this comment.
[Critical] Missing auth-type validation gate
The --fast handler (line 463) explicitly checks config.getContentGeneratorConfig()?.authType and returns an error when unavailable. This --compaction handler skips that gate entirely, going straight to resolveModelId(modelName).
When no auth type is configured, /model --compaction <id> produces a confusing error path instead of the clear "Authentication type not available." message that --fast shows.
| const isCompactionModelCommand = | |
| const isCompactionModelCommand = | |
| args === '--compaction' || args.startsWith('--compaction '); | |
| if (isCompactionModelCommand) { | |
| const contentGeneratorConfig = config.getContentGeneratorConfig(); | |
| const authType = contentGeneratorConfig?.authType; | |
| if (!authType) { | |
| return { | |
| type: 'message', | |
| messageType: 'error', | |
| content: t('Authentication type not available.'), | |
| }; | |
| } | |
| const modelName = args.replace('--compaction', '').trim(); |
— qwen3.7-max via Qwen Code /review
| }; | ||
| } | ||
|
|
||
| persistSetting(settings, 'compactionModel', modelName); |
There was a problem hiding this comment.
[Critical] Cross-provider model ambiguity — raw modelName persisted without authType qualifier
The --vision handler (line 614-617) builds qualifiedModelName = \${authType}:${modelId}`before persisting. This handler persists the baremodelName. When the same model ID (e.g., gpt-4o`) is configured under multiple providers, compression traffic could silently route to a different provider than the user intended.
| persistSetting(settings, 'compactionModel', modelName); | |
| const matched = availableModels.find((m) => m.id === selector.modelId); | |
| const qualifiedModelName = selector.authType | |
| ? `${selector.authType}:${selector.modelId}` | |
| : matched?.authType | |
| ? `${matched.authType}:${selector.modelId}` | |
| : modelName; | |
| persistSetting(settings, 'compactionModel', qualifiedModelName); | |
| config.setCompactionModel(qualifiedModelName); |
— qwen3.7-max via Qwen Code /review
| const [isFastModelMode, setIsFastModelMode] = useState(false); | ||
| const [isVoiceModelMode, setIsVoiceModelMode] = useState(false); | ||
| const [isVisionModelMode, setIsVisionModelMode] = useState(false); | ||
| const [isCompactionModelMode, setIsCompactionModelMode] = useState(false); |
There was a problem hiding this comment.
[Suggestion] Missing test coverage for compaction mode in useModelCommand.test.ts
The hook adds isCompactionModelMode with mutual-exclusion logic (compaction suppresses fast/voice/vision) and closeModelDialog resets it, but useModelCommand.test.ts has no test verifying:
openModelDialog({ compactionModelMode: true })setsisCompactionModelModeand suppresses other modescloseModelDialog()resetsisCompactionModelModeto false
Existing tests cover voice/vision modes; compaction should mirror that coverage.
— qwen3.7-max via Qwen Code /review
| this.autoSkillConfirm = params.autoSkillConfirm ?? true; | ||
| this.fastModel = params.fastModel || undefined; | ||
| this.visionModel = params.visionModel || undefined; | ||
| this.compactionModel = params.compactionModel || undefined; |
There was a problem hiding this comment.
[Suggestion] Duplicated resolution logic — resolveCompactionModelSelector() (line 3085) is a verbatim copy of resolveFastModelSelector() (line 3021)
Both methods share identical control flow: check field → resolveModelId → bare-name fallback via currentAuthType → getAllConfiguredModels. The only differences are this.fastModel vs this.compactionModel and the debug log string.
With three auxiliary model types (fast/vision/compaction), extracting a shared helper would prevent drift:
private resolveAuxModelSelector(fieldName: 'fastModel' | 'visionModel' | 'compactionModel', label: string) {
const value = this[fieldName];
if (!value) return undefined;
// ... shared logic ...
}— qwen3.7-max via Qwen Code /review
| isFastModelMode, | ||
| isVoiceModelMode, | ||
| isVisionModelMode, | ||
| isCompactionModelMode, |
There was a problem hiding this comment.
[Suggestion] Left-arrow dismiss and preferred-entry gaps for isCompactionModelMode
Two integration points missed beyond the existing gaps already noted on this component:
-
Left-arrow handler (~line 506):
(isFastModelMode || isVoiceModelMode || isVisionModelMode)omitsisCompactionModelMode. In all other specialized pickers, left-arrow dismisses the dialog. -
Preferred entry (~line 440): The preferredKey resolution chain has no
preferredCompactionModelEntry, so the current compaction model is never highlighted in the picker.
— qwen3.7-max via Qwen Code /review
|
@qwen-code /resolve |
There was a problem hiding this comment.
Pull request overview
Adds a new “compaction model” selection path to the CLI so users can configure a dedicated model used for chat auto-compaction (compression), with corresponding config plumbing and UI dialog support.
Changes:
- Extends
/modelwith--compaction(including completions, dialog routing, and persisted setting), and threads a newcompactionModelModethrough UI state/actions. - Adds
compactionModelto coreConfig(including runtime setter and resolution logic with fallback ordering). - Updates model-selection UI/types to support
visionOnlymodels and the new compaction dialog type.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/core/src/services/chatCompressionService.ts | Switches compression side-query model override logic (currently wired to fast model). |
| packages/core/src/services/chatCompressionService.test.ts | Updates compression tests to match the new model-selection behavior. |
| packages/core/src/models/types.ts | Adds visionOnly?: boolean to the AvailableModel shape. |
| packages/core/src/config/config.ts | Adds compactionModel param/state plus getCompactionModel() + setCompactionModel() resolution/update paths. |
| packages/cli/src/ui/hooks/useModelCommand.ts | Adds isCompactionModelMode state and option plumbing for opening the model dialog in compaction mode. |
| packages/cli/src/ui/hooks/slashCommandProcessor.ts | Routes compaction-model dialog open actions. |
| packages/cli/src/ui/contexts/UIStateContext.tsx | Extends UI state interface with isCompactionModelMode. |
| packages/cli/src/ui/contexts/UIActionsContext.tsx | Extends openModelDialog options with compactionModelMode. |
| packages/cli/src/ui/components/ModelDialog.tsx | Adds compaction-mode prop and adjusts model filtering behavior. |
| packages/cli/src/ui/components/DialogManager.tsx | Passes compaction-mode flag through to ModelDialog. |
| packages/cli/src/ui/commands/types.ts | Extends dialog union with 'compaction-model'. |
| packages/cli/src/ui/commands/modelCommand.ts | Implements /model --compaction handling, messages, and completion logic. |
| packages/cli/src/ui/commands/modelCommand.test.ts | Updates /model description assertion to include --compaction. |
| packages/cli/src/ui/AppContainer.tsx | Wires compaction mode into app UI state propagation. |
| packages/cli/src/config/settingsSchema.ts | Adds compactionModel setting definition/metadata. |
| packages/cli/src/config/config.ts | Passes compactionModel from CLI settings into core Config parameters. |
Comments suppressed due to low confidence (1)
packages/cli/src/ui/commands/modelCommand.ts:275
- getAvailableModelIds() does not account for the new visionOnly flag, so main/fast/voice/compaction completions can incorrectly include vision-only models. Filter visionOnly out for all modes except the vision selector.
if (mode === 'fast' || mode === 'compaction') return !m.voiceOnly;
if (mode === 'voice') return !m.fastOnly;
// 'vision' and 'main' both exclude fast/voice-only models.
return !m.fastOnly && !m.voiceOnly;
});
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| purpose: 'chat-compression', | ||
| skipOutputLanguagePreference: true, | ||
| model, | ||
| model: config.getFastModel?.(), | ||
| // Compression uses the fast model (config.getFastModel?.()) to reduce cost. | ||
| // See https://github.com/QwenLM/qwen-code/issues/5956 |
| getFastModel: vi.fn(), | ||
| getApprovalMode: () => 'default', |
| vi.mocked(mockChat.getHistory).mockReturnValue(history); | ||
| vi.mocked(uiTelemetryService.getLastPromptTokenCount).mockReturnValue(100); | ||
| vi.mocked(tokenLimit).mockReturnValue(1000); | ||
| vi.mocked(mockConfig.getFastModel).mockReturnValue('fast-model-v1'); |
| expect(mockGenerateText).toHaveBeenCalledWith( | ||
| expect.objectContaining({ | ||
| model: mockModel, | ||
| model: 'fast-model-v1', | ||
| maxAttempts: 1, |
| (isFastModelMode || !m.fastOnly) && | ||
| (isVoiceModelMode || !m.voiceOnly), | ||
| (isVoiceModelMode || !m.voiceOnly) && | ||
| (isVisionModelMode || !m.visionOnly) && | ||
| (isCompactionModelMode || !m.visionOnly), |
| content: t( | ||
| 'Current compaction model: {{compactionModel}}\nUse "/model --compaction <model-id>" to set compaction model, or "/model --compaction " to clear the override.', | ||
| { compactionModel }, | ||
| ), |
| const availableModels = ( | ||
| selector.authType | ||
| ? config.getAvailableModelsForAuthType(selector.authType) | ||
| : config.getAllConfiguredModels() | ||
| ).filter((m) => !m.voiceOnly); | ||
| if (!availableModels.some((model) => model.id === selector.modelId)) { |
|
Qwen Code resolved the merge conflicts and pushed the branch update. Merge Conflict Resolution Summary — PR #6019OverviewMerged Conflicts Resolved4 files had merge conflicts. All were independent additive conflicts — both sides added separate, non-overlapping features that should coexist. 1.
|
| } | ||
| const availableModels = config.getAvailableModels().filter((m) => { | ||
| if (mode === 'fast') return !m.voiceOnly; | ||
| if (mode === 'fast' || mode === 'compaction') return !m.voiceOnly; |
There was a problem hiding this comment.
[Suggestion] Model filter inconsistency across three compaction code paths
The model filter for compaction mode differs between the tab-completion path, the interactive dialog, and the execute handler:
| Code path | Filter | Excludes |
|---|---|---|
getAvailableModelIds (tab completion, this line) |
!m.voiceOnly |
voiceOnly only |
ModelDialog.tsx (interactive dialog) |
!m.fastOnly && !m.voiceOnly && !m.visionOnly |
all three |
| Execute handler (~line 690) | !m.voiceOnly |
voiceOnly only |
A user can tab-complete a fastOnly or visionOnly model with /model --compaction <TAB>, but the dialog would never show those models, and the execute handler would accept them.
Suggested fix: Align all three paths. If compaction is a text-summarization task that doesn't need specialized models, all three should exclude fastOnly, voiceOnly, and visionOnly:
| if (mode === 'fast' || mode === 'compaction') return !m.voiceOnly; | |
| if (mode === 'fast') return !m.voiceOnly; | |
| if (mode === 'compaction') return !m.voiceOnly && !m.fastOnly && !m.visionOnly; |
— qwen3.7-max via Qwen Code /review
| case 'vision-model': | ||
| actions.openModelDialog({ visionModelMode: true }); | ||
| return { type: 'handled' }; | ||
| case 'compaction-model': |
There was a problem hiding this comment.
[Suggestion] Missing test for compaction-model dialog dispatch
Parallel tests exist for 'voice-model' and 'vision-model' dialog cases in slashCommandProcessor.test.ts, verifying that openModelDialog is called with the correct mode flag. This new 'compaction-model' case is structurally identical but has no corresponding test.
Suggested fix: Add a test case that mocks a command returning { type: 'dialog', dialog: 'compaction-model' } and asserts openModelDialog is called with { compactionModelMode: true }.
— qwen3.7-max via Qwen Code /review
|
@qwen-code /resolve |
Resolve conflicts between the --compaction model mode feature and main's --project/--global scope persistence flags. Both features are orthogonal and coexist in all affected files: - useModelCommand.ts: include both isCompactionModelMode state and modelDialogPersistScope state in hook return type, callbacks, and closure - UIStateContext.tsx: add both isCompactionModelMode and modelDialogPersistScope to UIState interface - AppContainer.tsx: destructure and pass both values through to UIState consumers - modelCommand.ts: combine description, argumentHint, and completion logic for all flags (--fast, --voice, --vision, --compaction, --project, --global); adopt main's regex-based flag stripping for mode parsing extended with --compaction - modelCommand.test.ts: update expected description string to include all flags - DialogManager.tsx: pass both isCompactionModelMode and persistScope props to ModelDialog - ModelDialog.tsx: accept both isCompactionModelMode and persistScope in props interface and component destructuring - slashCommandProcessor.ts: include both compactionModelMode and persistScope in openModelDialog options type
|
Qwen Code resolved the merge conflicts and pushed the branch update. Merge Conflict Resolution: PR #6019Branch: OverviewPR #6019 adds a Conflict Summary8 files conflicted, 19 conflict blocks total. 1.
|
| # | Location | HEAD side | Main side | Resolution |
|---|---|---|---|---|
| 1 | Return type | isCompactionModelMode: boolean |
modelDialogPersistScope: ModelDialogPersistScope | undefined |
Both |
| 2 | openModelDialog param type |
compactionModelMode?: boolean |
persistScope?: ModelDialogPersistScope |
Both |
| 3 | State declarations | useState(false) for compaction |
useState<ModelDialogPersistScope | undefined>(undefined) for scope |
Both |
| 4 | Callback param type | Same as #2 | Same as #2 | Both |
| 5 | Callback body | Mutual exclusion logic for compaction mode | setModelDialogPersistScope(options?.persistScope) |
Both — HEAD's mutual exclusion preserved, main's scope setter appended |
| 6 | closeModelDialog |
setIsCompactionModelMode(false) |
setModelDialogPersistScope(undefined) |
Both |
| 7 | Return object | isCompactionModelMode |
modelDialogPersistScope |
Both |
2. packages/cli/src/ui/contexts/UIStateContext.tsx (1 conflict)
UIState interface definition. Added both `isCom
| }; | ||
| } | ||
| return { | ||
| type: 'dialog', |
There was a problem hiding this comment.
[Critical] --project/--global scope flags silently dropped in all three --compaction code paths
Unlike the --fast, --voice, and --vision handlers, the --compaction handler never propagates scopeOverride to persistence or the dialog:
- Line 758 — This dialog return is missing
...persistScopeSpread(scopeOverride)(compare--fastat line 552,--voiceat line 483,--visionat line 660). - Line 808 —
persistSetting(settings, 'compactionModel', modelName)is missing the 4th argumentscopeOverride(compare--fastat line 609:persistSetting(settings, 'fastModel', modelName, scopeOverride)). slashCommandProcessor.ts:850—actions.openModelDialog({ compactionModelMode: true })is missingpersistScope: result.persistScope(comparevision-modelcase which passes it).
Impact: Running /model --compaction gpt-4o --project silently saves to the default scope instead of workspace. The success message also omits scopeSuffix, so the user receives no feedback that --project was ignored.
| type: 'dialog', | |
| return { | |
| type: 'dialog', | |
| dialog: 'compaction-model', | |
| ...persistScopeSpread(scopeOverride), | |
| }; |
— qwen3.7-max via Qwen Code /review
|
@qwen-code /triage |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Thanks for the PR, @Rajeshwaran-R!
Template: the PR body is missing several required sections from the PR template: Reviewer Test Plan (with "How to verify", "Evidence (Before & After)", "Tested on"), Risk & Scope, and Linked Issues. Please update the PR description to follow the template so reviewers can verify your changes.
Critical bug: Config.getCompactionModel() (added in packages/core/src/config/config.ts) is defined with the right fallback chain (compactionModel → fastModel → main model), but it is never called anywhere. The compression service (chatCompressionService.ts:515) passes config.getFastModel?.() directly to runSideQuery, completely ignoring the user's compaction model setting. The entire /model --compaction feature — the UI, the persistence, the resolution logic — is dead code as far as the actual compression path is concerned. The fix is likely config.getCompactionModel?.() instead of config.getFastModel?.().
Other issues to address:
persistSetting(settings, 'compactionModel', modelName)is called withoutscopeOverride, so--projectand--globalflags have no effect when combined with--compaction. All other model types (fast, voice, vision) passscopeOverride.visionOnlyproperty was added toAvailableModelbut no provider ever sets it. The ModelDialog compaction filter(isCompactionModelMode || !m.visionOnly)is therefore a no-op — it never excludes any model. If compaction should hide vision-only models, this needs wiring; if not, the property addition is premature.- The ModelDialog filter reuses
visionOnlyfor compaction mode — this looks like a copy-paste error rather than intentional design.
Please fix the template, the dead-code bug, and the scopeOverride gap, then re-request review.
中文说明
感谢贡献,@Rajeshwaran-R!
模板: PR 描述缺少 PR 模板中的多个必需章节:Reviewer Test Plan(含 "How to verify"、"Evidence (Before & After)"、"Tested on")、Risk & Scope 和 Linked Issues。请按照模板更新 PR 描述,以便审查者验证变更。
严重 bug: Config.getCompactionModel()(在 packages/core/src/config/config.ts 中添加)定义了正确的回退链(compactionModel → fastModel → main model),但从未被任何地方调用。压缩服务(chatCompressionService.ts:515)直接将 config.getFastModel?.() 传给 runSideQuery,完全忽略了用户设置的压缩模型。整个 /model --compaction 功能——UI、持久化、解析逻辑——在实际压缩路径中都是死代码。修复方式可能是将 config.getFastModel?.() 改为 config.getCompactionModel?.()。
其他需要修复的问题:
persistSetting(settings, 'compactionModel', modelName)调用时缺少scopeOverride,因此--project和--global标志与--compaction组合时无效。其他所有模型类型(fast、voice、vision)都传递了scopeOverride。visionOnly属性被添加到AvailableModel,但没有任何 provider 设置它。ModelDialog 压缩过滤器(isCompactionModelMode || !m.visionOnly)因此是无效的——它从不排除任何模型。- ModelDialog 过滤器对压缩模式重复使用
visionOnly——看起来像是复制粘贴错误,而非有意设计。
请修复模板、死代码 bug 和 scopeOverride 缺失,然后重新请求审查。
— Qwen Code · qwen3.7-max
|
@qwen-code /triage |
|
Re-run (2026-07-07): no code changes since the last triage — all findings below remain valid and unresolved. Code ReviewCritical: compaction model is never used during compressionThe PR adds The result: a user can set a compaction model, see the confirmation in the UI, but compression will still use the fast model. The feature is configured but never consumed. Fix: Critical: ModelDialog vision-mode filter breaks
|
|
Re-run (2026-07-07): no code changes since the last triage. All findings below remain valid. ReflectionThe direction is sound — a dedicated compaction model is a natural extension of the But the feature has three critical correctness bugs that make it unsafe to ship:
Bug #3 alone is a hard blocker: the feature actively harms the user when invoked through the dialog. The PR also needs template compliance (Reviewer Test Plan, Risk & Scope, Linked Issues). Verdict: needs rework. The Config-layer plumbing is solid, but the service wiring and dialog integration are incomplete. Once the three critical bugs are fixed, the template is completed, and the feature is verified end-to-end, this should be a straightforward merge. Escalating to maintainer — this is a fork PR with correctness issues that need human judgment on whether the direction warrants iteration. 中文说明重跑 (2026-07-07): 自上次 triage 以来无代码变更。以下所有发现仍然有效。 反思方向正确——专用压缩模型是现有模式的自然扩展,Config 层实现设计良好。 但该功能有三个严重正确性 bug:
仅 Bug #3 就是硬阻塞。 判定: 需要返工。Config 层管线扎实,但服务接线和对话框集成不完整。修复后应可直接合并。 转交 maintainer——fork PR 有正确性问题,需要人工判断。 — Qwen Code · qwen3.7-max |
|
Re-run (2026-07-07): no code changes since the last triage — all findings below remain valid. Thanks for the PR, @Rajeshwaran-R! Template: the PR body is still missing required sections from the PR template: Reviewer Test Plan (with "How to verify", "Evidence (Before & After)", "Tested on"), Risk & Scope, and Linked Issues. This was flagged in the previous triage — please update the description. Problem: the motivation is reasonable — chat compression using the expensive primary model wastes tokens. The PR references issue #5956 in code comments, which is a positive signal, but no linked issue or reproduction in the PR body itself. Direction: aligned with the project's model-switching UX ( Size: 71 production core lines (69 additions, 2 deletions) plus 174 CLI lines. Well under 500 — no core-module gate concerns. Approach: the scope follows the established pattern for auxiliary model flags, but there's a critical wiring gap: the PR adds Additionally, the These need to be fixed before the PR can ship. 中文说明重跑 (2026-07-07): 自上次 triage 以来无代码变更——以下所有发现仍然有效。 感谢贡献! 模板: PR body 仍缺少 PR 模板 中的必需部分:Reviewer Test Plan(含"How to verify"、"Evidence (Before & After)"、"Tested on")、Risk & Scope 和 Linked Issues。上次 triage 已指出过——请更新描述。 问题: 动机合理——用昂贵的主模型做聊天压缩浪费 token。PR 代码注释中引用了 #5956,是正面信号,但 PR body 中没有关联 issue 或复现。 方向: 与项目现有的模型切换 UX( 规模: 71 行核心生产代码加 174 行 CLI 代码。远低于 500 行阈值,无核心模块门槛问题。 方案: 范围遵循已有的辅助模型 flag 模式,但存在 关键接线缺口:PR 添加了 此外, 这些需要在合并前修复。 — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Three critical correctness bugs found — see the Stage 2 and Stage 3 comments above for full details. Summary:
-
Compression service ignores the compaction model —
chatCompressionService.ts:514callsconfig.getFastModel?.()instead ofconfig.getCompactionModel?.(). The/model --compactionsetting is configured but never consumed during actual compression. -
ModelDialog filter breaks
/model --vision— the(isCompactionModelMode || !m.visionOnly)condition filters vision-only models out of the vision picker even when compaction mode is inactive (regression). -
Dialog selection writes to the wrong model —
handleSelecthas noisCompactionModelModebranch, so picking a model in the compaction dialog falls through toconfig.switchModel()and silently overwrites the primary model.
The Config-layer plumbing (getCompactionModel() fallback chain) is solid. These three integration bugs need to be fixed before this can ship. The PR template also needs completing (Reviewer Test Plan, Risk & Scope, Linked Issues).
— Qwen Code · qwen3.7-max
|
@qwen-code /triage |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Second-opinion review with 9 parallel agents (correctness, security, code quality, performance, test coverage, 3× undirected audit, build/test). All agents confirmed the existing inline comments are comprehensive — no new high-confidence issues found beyond what @doudouOUC, @DragonnZhang, @wenshao, and the CI bot have already flagged.
Build & test verification: All 155 tests pass (88 core + 67 CLI). Deterministic analysis (tsc + eslint) clean on changed files.
The critical blockers identified by prior reviewers are accurate and well-documented. Key items for the author to address:
chatCompressionService.ts:515— must callconfig.getCompactionModel()instead ofconfig.getFastModel?.()(the feature is otherwise non-functional)ModelDialog.tsx:316— copy-paste bug:!m.visionOnlyshould be!m.voiceOnlyin the compaction filterModelDialog.tsx—handleSelectmissingisCompactionModelModehandler (selecting a model falls through to primary model switch)modelCommand.ts—--project/--globalscope flags silently droppedmodels/types.ts—visionOnlyfield never populated in the model registry
— qwen3.7-max via Qwen Code /review
Adds the
--compactionflag to/model, letting users configure a dedicated model for chat compression (auto-compact).Changes:
--compactionflag on/modelcommand, opening a model dialog in compaction modecompactionModelModeoption in UIActionsContextopenModelDialog'compaction-model'dialog type in the Dialog union'Compaction model'label in unavailable model error messagesvisionOnlyproperty onAvailableModelinterface (used by ModelDialog)modelCommand.test.tsassertion for the new descriptionWhy: Chat compression can be expensive when using the primary model. Allowing a separate, cheaper model for compaction reduces token costs without affecting main conversation quality.
🤖 Generated with Qwen Code (2-shotted by Qwen-Coder)